Skip to content

fix(release): stamp .version at publish time so CLI reports the real tag (#1239)#1560

Open
ColinM-sys wants to merge 2 commits intoNVIDIA:mainfrom
ColinM-sys:fix/1239-version-stamp
Open

fix(release): stamp .version at publish time so CLI reports the real tag (#1239)#1560
ColinM-sys wants to merge 2 commits intoNVIDIA:mainfrom
ColinM-sys:fix/1239-version-stamp

Conversation

@ColinM-sys
Copy link
Copy Markdown
Contributor

@ColinM-sys ColinM-sys commented Apr 7, 2026

Summary

Closes #1239.

nemoclaw --version currently prints a stale hardcoded value (0.1.0) on every npm-installed copy, regardless of which tag was actually published.

Root cause

getVersion() in src/lib/version.ts tries three sources in order:

  1. git describe --tags --match 'v*' — works in dev clones, fails inside an npm install (no .git)
  2. .version file at repo root — meant to be stamped at publish time, but nothing ever writes it
  3. package.json version — hardcoded to 0.1.0 and not bumped per-release

So in production, steps 1 and 2 always fail and the CLI always reports 0.1.0.

Fix

Two minimal changes to package.json, no source changes:

  1. prepublishOnly now stamps .version from git describe --tags --match 'v*' before tsc runs.
  2. .version is added to the files array so it ships in the npm tarball.

The existing getVersion() chain already prefers .version over package.json (proven by the existing "prefers .version file over package.json" test), so no runtime code needs to change.

Test plan

  • Added a regression test in src/lib/version.test.ts mirroring the issue scenario: stale package.json (0.1.0) + correct .version (0.0.2) → asserts getVersion() returns 0.0.2.
  • vitest run src/lib/version.test.ts passes (4/4).
  • Manually verified locally: with .version present, getVersion() returns its contents; with it absent, falls through to git describe / package.json as before.

Why this approach over bumping package.json

Stamping a separate .version file:

  • Avoids coupling release tags to hand-edited package.json bumps
  • Doesn't conflict with whatever release tooling (release-please, semantic-release, etc.) the maintainers may adopt later
  • Is the same pattern used by uv, ruff, and other projects with the same git-tag-vs-package-version drift problem
  • Requires no changes to version.ts — leverages the fallback chain that already exists

Summary by CodeRabbit

  • Chores

    • Package publication now generates and includes a version metadata file in distributions, ensuring published packages contain explicit built version information.
  • Tests

    • Added a regression test that verifies the release process prefers the generated version metadata when present, preventing stale manifest versions from being used.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Apr 7, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 3abdae2f-972e-4d6f-9457-2a1204a1d433

📥 Commits

Reviewing files that changed from the base of the PR and between d8e2216 and 85240dc.

📒 Files selected for processing (2)
  • package.json
  • src/lib/version.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
  • src/lib/version.test.ts
  • package.json

📝 Walkthrough

Walkthrough

Prepublish now generates a .version file from git describe (stripping a leading v) and includes it in the published package; a test verifies getVersion() prefers .version over package.json when present.

Changes

Cohort / File(s) Summary
Version Stamping Configuration
package.json
prepublishOnly script: runs `git describe --tags --match 'v*'
Version Stamping Tests
src/lib/version.test.ts
Adds Vitest regression test that writes a stale package.json and a .version ("0.0.2") into the test dir, asserts getVersion({ rootDir }) returns .version, then restores state and cleans up.

Sequence Diagram(s)

sequenceDiagram
  participant Dev as Developer/CI
  participant Git as Git (tags)
  participant Prepub as prepublishOnly script
  participant Build as Build (npm/tsc)
  participant NPM as npm publish
  participant Install as Installed package
  participant Runtime as getVersion()

  Dev->>Git: trigger publish (tagged ref)
  Git-->>Prepub: git describe --tags --match 'v*'
  Prepub-->>Prepub: strip leading "v", write `.version`
  Prepub->>Prepub: test -s .version && install deps (--ignore-scripts) && run tsc
  Prepub->>NPM: publish package (includes `.version`)
  NPM-->>Install: package tarball (contains `.version`)
  Install->>Runtime: getVersion()
  alt `.version` exists
    Runtime-->>Install: return contents of `.version`
  else no `.version`
    Runtime-->>Install: fall back to `package.json` semver
  end
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐇 I sniffed the tags and found a clue,

I stamped a dot-file, tidy and true.
Now installs hop with versions right,
no stale semver in morning light.
A carrot for the CI crew 🥕

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically describes the main change: stamping a .version file at publish time to ensure the CLI reports the correct tagged version instead of a stale package.json value.
Linked Issues check ✅ Passed The PR fully addresses issue #1239 by implementing the required fix: stamping .version at publish time with git-tag-derived version and including it in npm package, enabling CLI to report correct installed version for npm-installed copies.
Out of Scope Changes check ✅ Passed All changes are directly scoped to issue #1239: package.json script/files modifications for .version stamping and a regression test validating the version precedence logic. No unrelated modifications present.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@package.json`:
- Line 19: The prepublishOnly script can create an empty .version when `git
describe` fails, causing getVersion() in src/lib/version.ts to fall back to
package.json and produce stale published versions; update the prepublishOnly
pipeline so the publish fails if tag resolution fails by ensuring the git
describe output is non-empty (for example capture output, test it with a shell
conditional and exit non-zero on empty, or append `|| exit 1` after git
describe), so .version is only written when a valid tag is obtained before
continuing with the nemoclaw install and tsc steps.

In `@src/lib/version.test.ts`:
- Line 37: Replace the GitHub URL in the inline comment within
src/lib/version.test.ts (the semver comment) with a plain issue reference like
"See issue `#1239`"; specifically edit the comment that currently reads "//
semver. See https://github.com/NVIDIA/NemoClaw/issues/1239" to remove the
external link and use "See issue `#1239`" (or equivalent plain text) so the test
file complies with the docs-link policy.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 9ac3e067-e314-472c-b045-0510dabd7e4e

📥 Commits

Reviewing files that changed from the base of the PR and between bbec268 and a99f7d8.

📒 Files selected for processing (2)
  • package.json
  • src/lib/version.test.ts

…real tag

The CLI's getVersion() chain already prefers a .version file over the
hardcoded package.json semver, but nothing in the publish pipeline ever
writes that file. As a result, npm-installed copies always fall through
to package.json's version (currently 0.1.0) regardless of which tag was
actually published, causing 'nemoclaw --version' to lie.

Stamp .version from 'git describe --tags' inside prepublishOnly and add
the file to the npm tarball via the 'files' field. No source changes —
the existing version.ts fallback chain already handles the file.

Adds a regression test that mirrors the issue scenario (stale package.json
+ correct .version) and asserts the .version value wins.

Closes NVIDIA#1239
@ColinM-sys ColinM-sys force-pushed the fix/1239-version-stamp branch from a99f7d8 to b57ce3f Compare April 7, 2026 04:15
ColinM-sys added a commit to ColinM-sys/NemoClaw that referenced this pull request Apr 7, 2026
… test comment

Address CodeRabbit review on NVIDIA#1560:
- The previous git describe | sed pipeline could write an empty .version
  if the repo had no matching tag, since pipes mask the upstream exit code.
  Add 'test -s .version' so prepublishOnly aborts when tag resolution fails
  rather than shipping a stale package.json version.
- Replace the third-party URL in the regression test comment with a plain
  issue reference to comply with the docs-link policy.
@wscurran wscurran added bug Something isn't working NemoClaw CLI Use this label to identify issues with the NemoClaw command-line interface (CLI). fix labels Apr 8, 2026
@wscurran
Copy link
Copy Markdown
Contributor

wscurran commented Apr 8, 2026

✨ Thanks for submitting this fix, which proposes a way to ensure the CLI reports the correct version by stamping it during publication.

Copy link
Copy Markdown
Contributor

@cv cv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM — security review PASS.

  • Version string from git describe --tags --match 'v*' is structurally constrained (alphanumeric + dots + hyphens)
  • test -s .version guard prevents publishing with empty/broken version
  • .version is gitignored — only lives in the npm tarball
  • buildVersionedUninstallUrl() strips the -N-gHASH suffix before URL interpolation
  • Fallback chain in getVersion() is unchanged — backwards compatible
  • No CI workflow changes

No concerns.

@cv cv added the v0.0.11 Release target label Apr 9, 2026
@ericksoa ericksoa added v0.0.12 Release target and removed v0.0.11 Release target labels Apr 10, 2026
… test comment

Address CodeRabbit review on NVIDIA#1560:
- The previous git describe | sed pipeline could write an empty .version
  if the repo had no matching tag, since pipes mask the upstream exit code.
  Add 'test -s .version' so prepublishOnly aborts when tag resolution fails
  rather than shipping a stale package.json version.
- Replace the third-party URL in the regression test comment with a plain
  issue reference to comply with the docs-link policy.

Signed-off-by: ColinM-sys <[email protected]>
@ColinM-sys ColinM-sys force-pushed the fix/1239-version-stamp branch from d8e2216 to 85240dc Compare April 10, 2026 01:18
@cv cv added v0.0.13 Release target and removed v0.0.12 Release target labels Apr 10, 2026
@prekshivyas prekshivyas self-assigned this Apr 10, 2026
@prekshivyas
Copy link
Copy Markdown
Contributor

@ColinM-sys pls resolve conflicts

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working fix NemoClaw CLI Use this label to identify issues with the NemoClaw command-line interface (CLI). v0.0.13 Release target

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[NemoClaw][all platform]nemoclaw --version reports package.json semver instead of installed git tag (e.g. v0.0.2 still shows 0.1.0)

5 participants